Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix scope for defineOwnProperties #1226

Closed
wants to merge 1 commit into from

Conversation

rbri
Copy link
Collaborator

@rbri rbri commented May 20, 2022

No description provided.

* @param o
* @param level
* @return boolean
* @see <a
* href="https://262.ecma-international.org/11.0/#sec-testintegritylevel">TestIntegrityLevel</a>
*/
static boolean testIntegrityLevel(Context cx, Object o, INTEGRITY_LEVEL level) {
static boolean testIntegrityLevel(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, the idea of all the methods in this class is that they implement the abstract object operations as defined by EcmaScript (with the addition of Context as first param): testIntegretyLevel in the EcmaScript specification doesn't have a scope parameter

Copy link
Collaborator Author

@rbri rbri May 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my point of view scope is like context, we need to add this to the params of many methods.

Copy link
Collaborator

@p-bakker p-bakker May 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the (top level) scope should be derivable from the ScriptableObject.

I think the issue here is that the prototype of the function is a NativeObject instance on which the parentScope isn;t set accordingly, hence it ends up being null in the ScriptableObject.getOwnPropertyDescriptor(...) method.

Adding obj.setParentScope(this); after the construction of the NativeObject instance in BaseFunction.setupDefaultPrototype() solves that. Then in ScriptableObject.getOwnPropertyDescriptor(...) the getParentSCope call starts working properly, by always returning a proper scope and the problem guess away

Did a quick test with this approach and it seems to solve the problem

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable, will have a look and maybe prepare another PR,

@rbri
Copy link
Collaborator Author

rbri commented May 21, 2022

Based on @p-bakker suggestion i made another one - #1227.

@rbri rbri closed this May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants